Skip to content

docs(claude): record the two CI-failure diagnosis traps that cost this session time - #1908

Open
seonghobae wants to merge 6 commits into
mainfrom
docs/claude-md-ci-diagnosis-clean
Open

docs(claude): record the two CI-failure diagnosis traps that cost this session time#1908
seonghobae wants to merge 6 commits into
mainfrom
docs/claude-md-ci-diagnosis-clean

Conversation

@seonghobae

Copy link
Copy Markdown
Contributor

Summary

My lane under the agreed multi-session work split (peer3 = verification discipline, peer1 = PR narrowing/ruling checks, peer2 = test & coverage gates, host1 = pre-close diff verification + noema concurrency, this = CI failure diagnosis). The split's rule is that a session only writes up what it actually hit and corrected first-hand; both bullets below are that.

1. A green local test run is not evidence the suite passes in CI. inspect_pr gates a real call on os.environ.get("GITHUB_ACTIONS") == "true". GitHub sets that variable for the entire job, including the pytest process running the suite — so the branch executes during tests in CI and never locally. 14 pre-existing tests that never anticipated the side effect failed only on the runner, blocking the agent-review-runtime-quality required check across ~20 unrelated PRs until it was found and fixed in #1896.

2. gh pr checks can report failures that are neither current nor real. It aggregates by check name and will keep surfacing an old cancelled run indefinitely — including the residual closure-event runs of an already-merged PR, which nothing will ever supersede. I lost real time tracking #1886's "failures" before discovering it had merged an hour earlier.

Verification

Both claims were checked against origin/main rather than from memory:

  • The guard exists verbatim at scripts/ci/pr_review_merge_scheduler_core.py:4168-4170 (recover_current_head_startup_failures(...) if not dry_run and os.environ.get("GITHUB_ACTIONS") == "true" else []).
  • The remediation advice ("stub the environment-gated call in the affected tests rather than changing the production guard") matches what the repo actually did: fix(tests): stub startup-failure recovery so scheduler tests are GITHUB_ACTIONS-agnostic #1896 (6d7fbebe) changed one file, +42 lines, tests only. On main there are now 15 multi-line stubs of that function, the 4 dedicated direct-call tests are intact, and no PYTEST_CURRENT_TEST workaround was introduced.

Per peer3's warning that same-model review misses inverted claims, I did not rely on my own reasoning for either statement — the evidence above is primary (grep of current main + the merged commit's own stat).

Merge order

Last in the sequence agreed with peers: #1907 (merged) → host1's #1906 → peer2 → peer1 → this. I will rebase as the earlier ones land.

Test plan

  • Full suite on this branch, rebased onto current main: 2883 passed, 1 skipped, 21 subtests
  • Single-file, docs-only diff (CLAUDE.md, +16)

🤖 Generated with Claude Code

…cost time on

Both are first-hand findings, assigned to this session under the peer work
split (peer3 takes verification discipline, peer1 queue operations):

1. GITHUB_ACTIONS-gated production code executes inside the test suite in CI
   but never locally, so "the suite passes on my machine" is not evidence.
   This blocked agent-review-runtime-quality across ~20 unrelated PRs (#1896).
2. gh pr checks surfaces stale cancelled runs indefinitely, including on
   already-merged PRs whose residual closure-event runs nothing supersedes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 5, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 40 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: df81279f-8c64-4a8f-9fb6-d6d9d4410f69

📥 Commits

Reviewing files that changed from the base of the PR and between f2f91b8 and d7ac615.

📒 Files selected for processing (1)
  • CLAUDE.md

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

seonghobae and others added 5 commits September 5, 2026 20:30
…e traps

Three measurements from the 2026-09-05 saturation work that are specific to
this organization and cost real time to establish.

Queue depth read from an unfiltered actions/runs call undercounts silently,
because that call returns only the ~30 most recent runs regardless of status.
Merge-gating read from classic branch protection alone misclassifies
ruleset-gated repositories, and reading either one without distinguishing a
central review context from a repository-local one (wardnet requires only
"rust", newsdom-api only "pytest") overstates how much of the queue can block
a merge. Enumerating all 76 repositories on both paths puts that figure at
five repositories, holding 39% of the queued runs.

The duplicate-job-name entry records a near miss: coverage-source-tree is an
echo-only sentinel in opencode-review.yml and a load-bearing job with
dependents in opencode-review-dispatch.yml. A truncated reference scan hid the
second one.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Correction to the previous commit, which asserted a queue-composition figure
derived from a defective query. Querying branches/main/protection across the
organization is wrong: 35 of the 76 repositories have a different default
branch. naruon has no main at all, so the API answers "Branch not found", and
reading that 404 as "unprotected" misclassified the second-largest queue
contributor. bandscope shows the other half of the trap, having a protected
main that is not its default, so a hard-coded query can return a plausible
answer for the wrong branch rather than an error.

Replaces the numeric claim with the query discipline, which is what transfers.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@seonghobae

Copy link
Copy Markdown
Contributor Author

Merged main@f2f91b80 into this head as d7ac6159 (parents e3ffb1eb + f2f91b80), no conflicts and no change to this PR's own content — so its next noema-review/strix runs are created after .github#1939 and execute the round-robin sidecar instead of the NVIDIA-only one behind the earlier 502 / provider-unavailable failures (a re-run would have kept the old workflow_sha). Verified before pushing: the merged tree is byte-identical to one an independent gate run passed (2896 passed / 1 skipped, coverage 100%, interrogate 100%) and contains no conflict markers. opencode-review and the CodeQL shards remain gated by .github#1929 and will not turn green from this.

@seonghobae seonghobae added documentation Improvements or additions to documentation priority: medium Normal-priority or P2 work status: needs-review Open pull request requiring current-head review or checks type: docs Documentation, ADR, PRD, or technical writing labels Sep 7, 2026 — with ChatGPT Codex Connector
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation priority: medium Normal-priority or P2 work status: needs-review Open pull request requiring current-head review or checks type: docs Documentation, ADR, PRD, or technical writing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant